-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Friendly errors fix #8224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev-2.0
Are you sure you want to change the base?
Friendly errors fix #8224
Conversation
test/unit/core/param_errors.js
Outdated
| test('set() with Boolean (invalid)', function () { | ||
| const result = mockP5Prototype._validate('p5.set', [0, 0, true]); | ||
| assert.equal(result.error, '🌸 p5.js says: Expected number or array or object at the third parameter, but received boolean in p5.set().'); | ||
| assert.equal(result.error, '🌸 p5.js says: Expected array or object at the third parameter, but received boolean in p5.set().'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was there a reason to alter the tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it passes the test cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Hi @Nitin2332 , thanks for your effort! Right now, your solution changes the tests to make them pass. The purpose of tests is to ensure stability: if you update this PR, make sure the tests are completely unchanged, and also that all tests pass. This requires using a different fix than the one you proposed. On your prior PR, @perminder-17 has provided some great explanations of how to address the problem. If you can revise this PR to implement those suggestions, I can review it. You are welcome to ask for clarification if you're not sure! However, if the code itself requires many additional iterations, to avoid blocking the project, I’m may have re-assign the issue / open it to other volunteers. |
069bdae to
d925a59
Compare
"resolves #8104"
PR Checklist
npm run lintpasses